Fix cplusplus.Move: remove access to moved-from object in StrongType test#37923
Fix cplusplus.Move: remove access to moved-from object in StrongType test#37923
Conversation
Co-authored-by: blozano-tt <181790211+blozano-tt@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a Clang Static Analyzer issue where a moved-from std::unique_ptr wrapped in StrongType was being accessed, which is undefined behavior. The fix removes the problematic assertion that attempted to verify the moved-from object's state, keeping only the verification of the moved-to object.
Changes:
- Removed two lines that accessed a moved-from
StrongType<std::unique_ptr<int>>object, eliminating undefined behavior - The test now correctly validates only the moved-to object contains the expected value
|
I agree with copilot, clang static analyzer, and clang-tidy. Let’s just delete this code. |
|
/codeowners ping |
CodeOwners Group AnalysisThis PR requires approval from one member of each of the following groups: Summary: 1 pending groups, 0 approved groups Group Information:
Note: At least one approval from each group is sufficient. |
|
Hi Audrey Kertesz (@akerteszTT), Artem Yerofieiev (@ayerofieiev-tt), this PR Fix cplusplus.Move: remove access to moved-from object in StrongType test by Copilot (@Copilot) needs your approval/review to merge this. |
Ticket
Link to Github Issue
Problem description
Clang Static Analyzer flags
cplusplus.Moveviolation intt_stl/tests/test_strong_type.cpp:78— test accessed a moved-fromStrongType<unique_ptr<int>>to verify the wrapped pointer was null. Accessing moved-from objects is undefined behavior per C++ standard.What's changed
Removed assertion that dereferenced the moved-from object. Test now only validates the moved-to object contains expected value, which sufficiently verifies move semantics without UB.
Before:
After:
Checklist
Model tests
If your changes cover model-related code, you should run tests corresponding to affected models and platforms (Single card, T3K, Galaxy). "Choose your pipeline" workflows facilitate running multiple kinds of tests in a single run. Each offers
models-mandatoryandmodels-extendedpresets.The former includes a minimal set of tests, to be run always. The latter extends that with additional ones - use your best judgement in deciding which is the most appropriate for your PR.
models-mandatorypreset (runs: Device perf regressions and Frequent model and ttnn tests)models-extendedpreset (runs: the mandatory tests, plus Demo and Model perf tests)models-mandatorypreset (runs: Unit tests)models-extendedpreset (runs: the mandatory tests, plus Demo and Model perf tests)models-mandatorypreset (runs: Quick tests)models-extendedpreset (runs: the mandatory tests, plus Demo and Model perf tests)Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.